Skip to content

Desktop app (GPUI) + durable chat history - #218

Open
jlucaso1 wants to merge 49 commits into
mainfrom
feat-call-ui
Open

Desktop app (GPUI) + durable chat history#218
jlucaso1 wants to merge 49 commits into
mainfrom
feat-call-ui

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Jan 6, 2026

Copy link
Copy Markdown
Collaborator

WhatsApp Desktop (GPUI)

This branch was rebuilt from scratch on current main. The original PR carried two things: a prototype call stack under src/calls (~17k lines) and the GPUI app. The call stack is obsolete — the library has since shipped a live-validated VoIP facade (#918) — so the PR now carries only the app, restructured around what the library provides today.

Layout: apps/desktop, a standalone nested workspace

The app lives in its own cargo workspace, listed in the root [workspace].exclude. The gpui dependency tree (~600 crates) and its pins stay in apps/desktop/Cargo.lock, out of the library's lockfile, CI and toolchain. The app consumes the library through path deps, so any library API change breaks the app build in the same PR that makes it — the app doubles as the library's consumer test (own path-filtered CI workflow).

No more gpui fork. The old branch patched gpui through jlucaso1/zed because cargo forbids [patch]-pinning a git source to itself. With a dedicated workspace the pin lives in the committed Cargo.lock instead (cargo update -p gpui --precise <rev>): everything resolves from upstream zed-industries/zed, at the exact rev gpui-component's own lockfile builds against.

Calls: library facade instead of an in-app media stack

accept/reject/call/hangup/mute go through client.voip(). Signaling, callKey crypto, relay connect, SRTP and the audio engine are all inside the library; the app supplies only a cpal mic/speaker bridge (adapted from the voip-cli example). The in-app RTP/SRTP/opus pipeline from the original branch (~1.4k lines) is deleted. This also removes the original PR's "call reconnection bug" surface — the facade's engine is the one validated in live calls.

Durable chat history (new)

The app now uses whatsapp-rust-chat-store (#1014: the crate materializes the client's event stream into the same SQLite file as the device store). For the app this means:

  • chats and messages survive restarts — the UI hydrates from the store at startup, before the socket even connects;
  • history sync (initial pairing backfill) is materialized, so the chat list populates with real history instead of only live traffic;
  • outgoing sends are recorded with delivery status tracked by acks/receipts.

This resolves the "messages are in-memory only" known issue from the original PR description, and most of "can't start new conversations" (chats now exist from history).

Ported to current APIs

Known limitations / follow-ups

  • Voice calls only: the library facade is 1:1 audio today; the video-call UI places voice calls.
  • Startup hydration restores text/captions; media bubbles re-download on demand (media-ref cache wiring is a follow-up — the store already keeps a hash→file index).
  • Reactions are persisted in the store but not yet hydrated into the UI at startup.
  • The cpal bridge is duplicated with the voip-cli example; worth extracting into a small shared crate if a third consumer appears.

Depends on #1014 (this branch includes its commit; it rebases clean once that merges).

@coderabbitai

coderabbitai Bot commented Jan 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a standalone GPUI WhatsApp desktop client with workspace isolation, WhatsApp event integration, chat and call state, responsive virtualized UI, audio/video media handling, pairing views, and desktop CI checks.

Changes

Desktop application

Layer / File(s) Summary
Workspace and application scaffold
.github/workflows/desktop.yml, .gitignore, Cargo.toml, apps/desktop/...
Adds the standalone Cargo workspace, pinned CI workflow, documentation, startup entry point, embedded assets, theme constants, responsive layout, and utilities.
State, layout, and caches
apps/desktop/src/state/*, apps/desktop/src/app/{calls,chats,media,messages}.rs, apps/desktop/src/responsive.rs
Defines lifecycle, chat/message/media/call state, UI events, responsive sizing, and virtual-list caches.
WhatsApp client integration
apps/desktop/src/client/*
Runs the WhatsApp client runtime, hydrates durable history, translates events, handles media downloads, sends messages and audio, and controls calls.
Audio and video pipelines
apps/desktop/src/audio/*, apps/desktop/src/video/*
Adds CPAL capture/playback, PTT recording and Opus/OGG encoding, waveform generation, MP4 audio extraction, H.264 decoding, and media playback state.
Application orchestration
apps/desktop/src/app/mod.rs
Wires event handling, navigation, cache invalidation, input actions, calls, recording, audio/video playback, and lifecycle rendering.
Reusable UI and views
apps/desktop/src/components/*, apps/desktop/src/views/*
Adds virtualized chat/message rendering, typing and send controls, media controls, call popups, loading/error screens, and QR pairing UI.

Estimated code review effort: 5 (Critical) | ~180 minutes

Possibly related PRs

Suggested labels: api-design

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and matches the main change: the GPUI desktop app with durable chat history.
Description check ✅ Passed The description is directly related to the changeset and accurately describes the desktop app rebuild and durable history work.
Docstring Coverage ✅ Passed Docstring coverage is 98.26% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-call-ui

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jlucaso1 jlucaso1 changed the title Feat call UI Feat call + UI Jan 8, 2026
@Salientekill

Copy link
Copy Markdown
Contributor

isso aqui vai dar uma dor de cabeça.......

@jlucaso1

Copy link
Copy Markdown
Collaborator Author

This PR zed-industries/zed#46758 will simplify things about video rendering :)

@rsaisankalp

Copy link
Copy Markdown

is this implmented whatsapp voice call where is it stuck the , is the call working reading the stream and playing the audio stream working

@joaosouz4dev

Copy link
Copy Markdown

this is amazing

@github-actions

github-actions Bot commented Apr 21, 2026

Copy link
Copy Markdown

Benchmark Results

59 unchanged benchmark(s)
Benchmark Current Baseline Change
reporting_token_benchmark::content_extraction_group::bench_content_extraction simple:setup_simple_message() 3,933 3,933 +0.0%
reporting_token_benchmark::content_extraction_group::bench_content_extraction extended:setup_extended_message() 12,038 12,038 +0.0%
reporting_token_benchmark::key_derivation_group::bench_key_derivation 43,514 43,514 +0.0%
reporting_token_benchmark::token_calculation_group::bench_token_calculation 19,365 19,365 +0.0%
reporting_token_benchmark::full_generation_group::bench_full_token_generation simple:setup_full_gen_simple() 68,579 68,579 +0.0%
reporting_token_benchmark::full_generation_group::bench_full_token_generation extended:setup_full_gen_extended() 76,679 76,679 +0.0%
reporting_token_benchmark::message_encoding_group::bench_message_encoding simple:setup_simple_message() 2,230 2,230 +0.0%
reporting_token_benchmark::message_encoding_group::bench_message_encoding extended:setup_extended_message() 5,988 5,988 +0.0%
send_receive_benchmark::dm_send::bench_dm_send text:setup_dm_send() 169,073 169,073 +0.0%
send_receive_benchmark::dm_recv::bench_dm_recv text:setup_dm_recv() 190,986 190,838 +0.1%
send_receive_benchmark::group_send::bench_group_send group_10:setup_group_send_10() 875,119 875,217 -0.0%
send_receive_benchmark::group_send::bench_group_send group_50:setup_group_send_50() 966,187 965,458 +0.1%
send_receive_benchmark::group_send::bench_group_send group_256:setup_group_send_256() 1,453,294 1,453,286 +0.0%
send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_10:setup_group_skdm_10() 2,575,016 2,568,483 +0.3%
send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_50:setup_group_skdm_50() 9,340,288 9,375,945 -0.4%
send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_256:setup_group_skdm_256() 44,454,724 44,455,417 -0.0%
send_receive_benchmark::group_recv::bench_group_recv text:setup_group_recv() 12,650,251 12,602,765 +0.4%
binary_benchmark::marshal_group::bench_marshal_allocating 71,247 71,247 +0.0%
binary_benchmark::marshal_group::bench_marshal_auto_allocating 71,300 71,300 +0.0%
binary_benchmark::marshal_group::bench_marshal_exact_allocating 98,367 98,367 +0.0%
binary_benchmark::marshal_group::bench_marshal_reusing_buffer 78,801 78,801 +0.0%
binary_benchmark::marshal_group::bench_marshal_reusing_buffer_vec_writer 71,347 71,347 +0.0%
binary_benchmark::marshal_group::bench_marshal_long_string 7,518 7,518 +0.0%
binary_benchmark::marshal_group::bench_marshal_auto_long_string 7,561 7,561 +0.0%
binary_benchmark::marshal_group::bench_marshal_exact_long_string 9,273 9,273 +0.0%
binary_benchmark::marshal_group::bench_marshal_huge_bytes_allocating 530,504 530,504 +0.0%
binary_benchmark::marshal_group::bench_marshal_auto_huge_bytes_allocating 530,072 530,072 +0.0%
binary_benchmark::marshal_group::bench_marshal_exact_huge_bytes_allocating 531,427 531,427 +0.0%
binary_benchmark::marshal_group::bench_marshal_many_children_allocating 8,506,160 8,506,160 +0.0%
binary_benchmark::marshal_group::bench_marshal_auto_many_children_allocating 8,450,412 8,450,412 +0.0%
binary_benchmark::marshal_group::bench_marshal_exact_many_children_allocating 19,677,947 19,677,947 +0.0%
binary_benchmark::unmarshal_group::bench_unmarshal small:setup_small_marshaled() 2,468 2,468 +0.0%
binary_benchmark::unmarshal_group::bench_unmarshal large:setup_large_marshaled() 33,558 33,558 +0.0%
binary_benchmark::unpack_group::bench_unpack_uncompressed 787 787 +0.0%
binary_benchmark::unpack_group::bench_unpack_compressed 526,732 526,732 +0.0%
binary_benchmark::attr_parser_group::bench_attr_parser attr_lookup:setup_attr_marshaled() 4,986 4,986 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip small:setup_small_marshaled() 5,315 5,315 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip large:setup_large_marshaled() 61,874 61,874 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip_auto small:setup_small_marshaled() 5,347 5,347 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip_auto large:setup_large_marshaled() 61,942 61,942 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip_exact small:setup_small_marshaled() 6,734 6,734 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip_exact large:setup_large_marshaled() 85,564 85,564 +0.0%
binary_benchmark::child_iteration_group::bench_get_children_by_tag 477,570 477,570 +0.0%
binary_benchmark::jid_optimization_group::bench_jid_to_owned_access jid_access:setup_jid_heavy_marshaled() 11,563 11,563 +0.0%
libsignal_benchmark::dm_group::bench_dm_session_establishment setup:setup_dm_users() 17,329,142 17,369,805 -0.2%
libsignal_benchmark::dm_group::bench_dm_encrypt_first_message first_msg:setup_dm_session() 157,113 157,113 +0.0%
libsignal_benchmark::dm_group::bench_dm_decrypt_first_message decrypt_prekey:setup_dm_with_first_message() 5,510,200 5,510,200 +0.0%
libsignal_benchmark::dm_group::bench_dm_encrypt_subsequent_message subsequent:setup_established_dm_session() 157,827 157,827 +0.0%
libsignal_benchmark::group_messaging_group::bench_group_create_distribution_message create:setup_group_sender() 296,767 296,767 +0.0%
libsignal_benchmark::group_messaging_group::bench_group_encrypt_message encrypt:setup_group_with_distribution() 706,282 706,282 +0.0%
libsignal_benchmark::group_messaging_group::bench_group_decrypt_message decrypt:setup_group_with_encrypted_message() 12,537,601 12,576,971 -0.3%
libsignal_benchmark::conversation_group::bench_full_dm_conversation full:setup_conversation_data() 27,448,723 27,664,863 -0.8%
libsignal_benchmark::signature_group::bench_signature_creation sign:setup_keypair_with_message() 3,467,011 3,467,011 +0.0%
libsignal_benchmark::signature_group::bench_signature_verification verify:setup_keypair_with_message() 126,529,253 125,448,533 +0.9%
libsignal_benchmark::signature_group::bench_key_generation keygen 2,830,452 2,830,452 +0.0%
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions() 46,003 46,003 +0.0%
libsignal_benchmark::session_optimization_group::bench_out_of_order_decryption out_of_order:setup_out_of_order_messages() 5,072,844 5,072,844 +0.0%
libsignal_benchmark::session_optimization_group::bench_promote_matching_session promote:setup_promote_matching_session() 316,083 316,083 +0.0%
libsignal_benchmark::session_optimization_group::bench_message_key_eviction eviction:setup_message_key_eviction() 14,255,917 14,255,917 +0.0%
No significant changes detected.

@jlucaso1

Copy link
Copy Markdown
Collaborator Author

Finally got calls working in: #918

@Mte90

Mte90 commented Jul 9, 2026

Copy link
Copy Markdown

this branch is usable? I don't want to use the web client and I just need the chat.

@jlucaso1 jlucaso1 changed the title Feat call + UI Desktop app (GPUI) + durable chat history Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

📦 Binary size report

Metric main PR Δ
bin size (stripped) 10.86 MiB 10.86 MiB +1.91 KiB (+0.02%) 🔺
bin .text 8.86 MiB 8.86 MiB +1.88 KiB (+0.02%) 🔺
bin allocated (text+data+bss) 10.86 MiB 10.86 MiB +4.05 KiB (+0.04%) 🔺
llvm-lines wacore 505,737 506,145 +408 (+0.08%) 🔺
llvm-lines wacore copies 17,371 17,380 +9 (+0.05%) 🔺
llvm-lines whatsapp-rust lib 772,606 772,541 -65 (-0.01%) 🔽
llvm-lines whatsapp-rust lib copies 25,079 25,079 0
deps crates (Cargo.lock) 472 472 0
.text per crate
Crate main PR Δ
.text whatsapp_rust 1.68 MiB 1.68 MiB -180 B (-0.01%) 🔽
.text wacore 527.79 KiB 530.04 KiB +2.25 KiB (+0.43%) 🔺
.text wacore_binary 148.45 KiB 148.27 KiB -185 B (-0.12%) 🔽
.text wacore_libsignal 179.42 KiB 179.42 KiB 0
.text wacore_appstate 158.25 KiB 158.25 KiB 0
.text wacore_noise 26.05 KiB 26.05 KiB 0
.text waproto 1.60 MiB 1.60 MiB 0
.text whatsapp_rust_sqlite_storage 513.00 KiB 513.00 KiB 0
.text whatsapp_rust_tokio_transport 43.79 KiB 43.79 KiB 0
.text whatsapp_rust_ureq_http_client 10.47 KiB 10.47 KiB 0
.text std 1.01 MiB 1.01 MiB 0
.text other deps 2.95 MiB 2.95 MiB 0
Top movers (cargo-bloat attribution)
Crate main PR Δ
wacore 527.79 KiB 530.04 KiB +2.25 KiB (+0.43%)
rustix 1.88 KiB 191 B -1.69 KiB (-90.08%)
buffa_descriptor 2.98 KiB 4.67 KiB +1.69 KiB (+56.87%)

Baseline: 9476f3752 (latest main run) · Head: 87b6a756d · Graphs

@jlucaso1
jlucaso1 marked this pull request as ready for review July 9, 2026 18:17
@coderabbitai coderabbitai Bot added api-design size-increase-ok Accepted binary-size increase: downgrades the per-PR size gate to a warning labels Jul 9, 2026
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a standalone GPUI desktop app (apps/desktop) for the WhatsApp Rust library, backed by the new whatsapp-rust-chat-store crate for durable SQLite chat history. The app consumes the library via path dependencies, doubles as a consumer-build test, and shares the same database file with the device store.

  • Desktop app (apps/desktop): Full GPUI UI with chat list, message bubbles, voice messages (PTT), video playback, call support (voice/video UI), and lazy media downloads. Chat history, reactions, and delivery status now survive restarts by hydrating from the chat store on startup.
  • Chat store enhancements: New mark_send_failed writer path, marked_as_unread + username field support in history sync, and per-conversation PN/LID mapping extraction alongside the bulk mapping block.
  • History sync (wacore): extract_conversation_fields now collects per-conversation PN↔LID pairs that the bulk phoneNumberToLidMappings block can miss, ensuring proper chat identity normalization at startup.

Confidence Score: 4/5

Safe to merge as a feature branch; the new desktop app and chat-store additions are architecturally sound and the previously raised bugs show fixes committed in subsequent SHAs. No new blocking issues found.

All previously-flagged blocking correctness issues (send failure persistence, ID reconciliation, receipt cache staleness, prune-vs-limit, cancel-with-placeholder-ID, duplicate insert) show developer replies confirming fixes in commits 3b6202a, dad25e6, fcd7bc1, 2ab9c08, 786dfb6. The only new findings are two UX-level gaps: document download has no UI feedback, and video player eviction is non-deterministic. Neither corrupts state or breaks the core messaging flow.

apps/desktop/src/app/mod.rs — document download task discards the entity/context handles, leaving users with no success or failure feedback. apps/desktop/src/client/whatsapp.rs — the eager image/sticker download inside the event handler loop (flagged in a previous thread) is still present in the diff.

Important Files Changed

Filename Overview
apps/desktop/src/client/whatsapp.rs Core event pump and UI bridge; image/sticker eager downloads block the event handler (previous thread), and the _client naming on try_extract_media is misleading since the client IS used. Multiple previously-flagged send-path issues (JID parse failure, send failure persistence, outgoing ID reconciliation) show replies indicating fixes landed.
apps/desktop/src/app/mod.rs Main UI state machine; previously-flagged cache stale issues (select_chat, receipt events) are now fixed with explicit invalidate_message_cache/invalidate_chat_cache calls. download_document emits no UI notification on success/failure — _entity and _cx are both unused in the spawned task.
apps/desktop/src/state/chat.rs Chat/message state with sorted dedup-guarded insert, rename_message remove-and-reinsert, and insert_history_message media-byte grafting. Well-tested; the previous duplicate-insert bug is correctly fixed with an Ok(pos) early return in add_message.
storages/chat-store/src/store.rs New SendFailed writer message correctly guards against regressing a positive ack (Pending-only filter). apply_history_conversation now sets UNREAD_MARKER when marked_as_unread=true, discarding the real unread count in that case — likely intentional per WA semantics.
wacore/src/history_sync.rs extract_conversation_fields now harvests per-conversation PN/LID pairs (fields 39/42) alongside the bulk mapping block, with wire-tag asserts. history_lid_mapping extracted as a shared helper. Clean refactor with no logic regressions.
.github/workflows/desktop.yml New path-filtered CI workflow for the desktop workspace. Installs Linux audio/display system dependencies and protoc; runs fmt, clippy, and tests.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI as WhatsAppApp (GPUI)
    participant WA as WhatsAppClient
    participant Store as ChatStore (SQLite)
    participant WS as WhatsApp Server

    Note over UI,WS: Startup / history hydration
    UI->>WA: new() + start()
    WA->>Store: SqliteStore::new + ChatStore::new
    WA->>Store: load_history (top-100 chats, 50 msgs each)
    Store-->>WA: "Vec<Chat> + complete flag"
    WA-->>UI: "UiEvent::HistoryLoaded {chats, complete}"
    UI->>UI: merge/prune chat list
    WA->>WS: bot.run() (connect)
    WS-->>WA: Event::Connected
    WA-->>UI: UiEvent::Connected

    Note over UI,WS: Live message receive
    WS-->>WA: Event::Messages(batch)
    WA->>WA: try_extract_media (eager download for images/stickers)
    WA-->>UI: UiEvent::MessageReceived
    UI->>UI: add_message + invalidate_caches
    Store->>Store: apply_event (writer queue)
    Store-->>WA: StoreChange::Messages
    WA->>Store: load_history (debounced 200ms)
    WA-->>UI: UiEvent::HistoryLoaded (merge)

    Note over UI,WS: Outgoing message
    UI->>WA: send_message(jid, text, local_id)
    UI->>UI: add optimistic bubble (local_id)
    WA->>WA: "generate_message_id -> msg_id"
    WA-->>UI: "UiEvent::MessageIdAssigned (local_id->msg_id)"
    WA->>Store: record_outgoing (PENDING)
    WA->>WS: send_message_with_options
    WS-->>WA: ServerAck
    Store->>Store: "apply_server_ack (PENDING->SERVER_ACK)"
    WS-->>WA: Event::Receipt(Read)
    WA-->>UI: UiEvent::ReceiptReceived
    UI->>UI: mark_messages_as_read + invalidate_caches
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI as WhatsAppApp (GPUI)
    participant WA as WhatsAppClient
    participant Store as ChatStore (SQLite)
    participant WS as WhatsApp Server

    Note over UI,WS: Startup / history hydration
    UI->>WA: new() + start()
    WA->>Store: SqliteStore::new + ChatStore::new
    WA->>Store: load_history (top-100 chats, 50 msgs each)
    Store-->>WA: "Vec<Chat> + complete flag"
    WA-->>UI: "UiEvent::HistoryLoaded {chats, complete}"
    UI->>UI: merge/prune chat list
    WA->>WS: bot.run() (connect)
    WS-->>WA: Event::Connected
    WA-->>UI: UiEvent::Connected

    Note over UI,WS: Live message receive
    WS-->>WA: Event::Messages(batch)
    WA->>WA: try_extract_media (eager download for images/stickers)
    WA-->>UI: UiEvent::MessageReceived
    UI->>UI: add_message + invalidate_caches
    Store->>Store: apply_event (writer queue)
    Store-->>WA: StoreChange::Messages
    WA->>Store: load_history (debounced 200ms)
    WA-->>UI: UiEvent::HistoryLoaded (merge)

    Note over UI,WS: Outgoing message
    UI->>WA: send_message(jid, text, local_id)
    UI->>UI: add optimistic bubble (local_id)
    WA->>WA: "generate_message_id -> msg_id"
    WA-->>UI: "UiEvent::MessageIdAssigned (local_id->msg_id)"
    WA->>Store: record_outgoing (PENDING)
    WA->>WS: send_message_with_options
    WS-->>WA: ServerAck
    Store->>Store: "apply_server_ack (PENDING->SERVER_ACK)"
    WS-->>WA: Event::Receipt(Read)
    WA-->>UI: UiEvent::ReceiptReceived
    UI->>UI: mark_messages_as_read + invalidate_caches
Loading

Reviews (33): Last reviewed commit: "docs(history): lid_mappings collects bul..." | Re-trigger Greptile

Comment thread storages/chat-store/src/store.rs Outdated
Comment thread apps/desktop/src/app/mod.rs Outdated
Comment thread apps/desktop/src/app/mod.rs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8f70a14116

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread storages/chat-store/src/store.rs Outdated
Comment thread storages/chat-store/src/store.rs
Comment thread apps/desktop/src/client/whatsapp.rs Outdated
Comment thread apps/desktop/src/audio/recorder.rs Outdated
Comment thread apps/desktop/src/audio/player.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 31

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/desktop.yml:
- Around line 34-62: The desktop workflow is missing run deduplication, so every
update can start a new job even when an older one is still running. Add a
workflow-level concurrency block to the desktop job definition in desktop.yml,
using a stable group based on the workflow name and branch/PR ref and enabling
cancellation of in-progress runs. This should be placed alongside the existing
top-level jobs/desktop configuration so the actions for checkout,
rust-toolchain, and cargo checks inherit it automatically.

In `@apps/desktop/README.md`:
- Line 15: The fenced code blocks in the README are missing language specifiers,
which triggers markdownlint MD040. Update the architecture tree block and the
data flow diagram block to use a text language tag so the markdown renderer and
linting both treat them correctly. Make the change in the README’s fenced blocks
identified by the architecture tree and the data flow diagram sections.
- Around line 15-47: The architecture tree in README.md is out of date, so
update the `apps/desktop/src` layout to reflect the real module structure.
Replace the single-file `app.rs` entry with the `app/` module directory and its
actual submodules (`app/mod.rs`, `app/calls.rs`, `app/chats.rs`,
`app/media/mod.rs`, `app/messages.rs`), and fix the components listing so
`input_area.rs` becomes `input_area_view.rs`. Keep the rest of the tree aligned
with the current Rust module/file names, using the existing section around
`components` and `app` as the reference points.

In `@apps/desktop/src/app/calls.rs`:
- Around line 30-32: The set_incoming method in Calls currently overwrites any
existing pending IncomingCall without notice, which can orphan the earlier offer
in CallRegistry.pending. Update set_incoming to detect when self.incoming is
already Some and either reject the new incoming call or at least log a warning
before replacing it, using the set_incoming and CallRegistry.pending symbols to
keep the behavior safe and traceable.

In `@apps/desktop/src/app/messages.rs`:
- Around line 73-124: The hard height cap at the end of calculate_message_height
is causing the virtual list to allocate too little space for long ChatMessage
entries. Remove the 500px clamp, or replace it with a much larger bound that
won’t truncate realistic message heights, so the returned value always matches
the actual bubble size. Keep the rest of the sizing logic in
calculate_message_height unchanged and ensure the final height reflects content,
media, sender name, and reactions without artificial clipping.

In `@apps/desktop/src/app/mod.rs`:
- Around line 1263-1264: The video decoding path in `StreamingVideoDecoder::new`
is using `smol::unblock` for heavy CPU work, which violates the blocking-work
guideline. Update the decode call site to use `tokio::task::spawn_blocking`
instead, keeping the `decode_result` flow intact and preserving the surrounding
async handling in `mod.rs`. If this code must remain on GPUI’s smol runtime,
bridge the work to Tokio explicitly or confirm the guideline should be relaxed
for this context.
- Around line 1156-1194: The paused-video resume path in the
VideoPlayerState::Paused branch is clearing the audio state too early, which
prevents the existing audio from being resumed. Update the logic around
self.stop_current_media() so it is skipped when self.audio_owner already matches
the current message_id, and keep the current self.audio_player.resume() path in
place for that case. Use the VideoPlayerState::Paused branch,
stop_current_media(), and audio_owner check to locate the fix.

In `@apps/desktop/src/audio/player.rs`:
- Around line 242-249: The Opus decode path in player::decode_float is
truncating stereo PCM as if the returned sample count were mono. Update the
logic in the decode result handling so that output.truncate uses the full frame
count across channels (or otherwise size the buffer/slice based on the channel
count) before extending all_samples, ensuring 2-channel audio keeps both
channels instead of dropping half the decoded data.
- Around line 238-240: The lazy decoder initialization in the player runtime
path is panicking via `OpusDecoder::new(...).expect("default decoder")`; replace
this with error propagation so `decoder.get_or_insert_with` no longer assumes
success. Update the `PlayerError::DecodeError` handling in `player.rs` so
decoder creation failures are returned to the caller the same way the header
branch already does, and keep the fix localized around the `OpusDecoder`
initialization logic.

In `@apps/desktop/src/client/whatsapp.rs`:
- Around line 147-165: Remove the redundant UI sender initialization in the
whatsapp client startup flow: the `self.runtime.spawn` block that only locks
`ui_sender_clone` and assigns `ui_tx_clone` duplicates the sender storage
already done in `run_client`. Keep the sender stored once in the async path that
calls `Self::run_client`, and delete the extra task to avoid unnecessary work
and any race between the two assignments.
- Around line 1189-1199: normalize_chat_jid is hitting client.get_lid_pn_entry
for every PN JID, creating an N+1 database lookup path for incoming messages and
receipts. Refactor normalize_chat_jid to avoid repeated queries by adding an
in-memory PN→LID cache (for example, a HashMap guarded by a Mutex or similar
shared state) and consult it before calling get_lid_pn_entry; update the cache
on successful lookups so repeated JIDs reuse the stored mapping.
- Line 180: The backend initialization in the whatsapp client uses a hardcoded
SqliteStore::new("whatsapp.db") path, which is not portable across launch
contexts. Update the code around the backend setup to derive the database
location from a proper platform data directory (for example via
dirs::data_dir()) or pass the path into the initializer as a parameter, and keep
the path handling in the same area as the SqliteStore::new call so it can be
configured per environment.
- Around line 1153-1170: The `stored_to_chat_message` helper is hardcoding
`is_read` to true for every `StoredMessage`, which conflicts with the chat-level
unread state loaded elsewhere. Update this conversion to use the stored
message’s read status if the `whatsapp_rust_chat_store::StoredMessage` provides
one, or otherwise derive a safer default that keeps messages unread when the
chat has pending unread items. Keep the fix localized to
`stored_to_chat_message` and preserve the existing mapping for other
`ChatMessage` fields.
- Around line 628-667: `send_message` is using `std::thread::spawn` with
`runtime.block_on`, and it keeps the `client_handle` lock held across the async
send. Update this method to follow the `set_call_muted`/`accept_call` pattern:
schedule work with `runtime.spawn`, clone the `Client` out of `client_handle`
and release the lock before awaiting, then perform `send_message` and
`record_outgoing` inside the spawned async task. Apply the same pattern to the
other public methods named in the comment for consistency.
- Around line 95-100: The Tokio runtime initialization in WhatsApp client setup
uses an .expect() panic path, which violates the no-panics guideline. Update the
constructor in whatsapp::new to return a Result instead of assuming runtime
creation succeeds, and propagate the error from
tokio::runtime::Builder::new_multi_thread().build() back to the caller with
proper error handling rather than unwrapping.

In `@apps/desktop/src/components/avatar.rs`:
- Around line 13-26: The avatar constructors are duplicated: from_initial and
render both build the same GpuiAvatar with identical sizing logic. Update
from_initial to delegate to render by converting the char to a string and
passing it through render, so all avatar construction stays centralized in the
render function and future changes only need to be made in one place.

In `@apps/desktop/src/components/input_area_view.rs`:
- Around line 170-172: `set_recording` in `InputAreaView` only mutates
`is_recording`, so the PTT UI never re-renders when `render()` depends on that
flag for icon, color, and variant. Update `set_recording` to accept `cx` and
call `cx.notify()` after changing the field, so callers handling
`StartRecording`/`StopRecording` can refresh the component without relying on
parent state changes.

In `@apps/desktop/src/main.rs`:
- Line 59: The `main` startup path in `apps/desktop/src/main.rs` still uses
`.unwrap()` on the window creation result, which violates the no-unwrap
guideline. Update the error handling at the call site in `main` so failures are
handled explicitly: either replace `.unwrap()` with `.expect()` using a clear,
descriptive message, or better, match on the result, log the failure, and exit
gracefully. Use the surrounding `main`/window creation code to keep the fix
localized and avoid any panic-only behavior without context.

In `@apps/desktop/src/state/chat.rs`:
- Around line 333-363: The unread-count logic in `ChatState::add_message` is too
permissive because `is_newer` is reused for both unread tracking and preview
updates, causing same-timestamp history messages to increment `unread_count`.
Split the conditions so the unread increment only happens for incoming messages
with a strictly newer timestamp than `last_message_time`, while the
`last_message` and `last_message_time` preview update can continue to use the
current inclusive behavior. Keep the fix localized in `add_message` and preserve
the existing chronological insert logic.

In `@apps/desktop/src/utils.rs`:
- Around line 7-16: The fallback in mime_to_image_format silently maps unknown
MIME types to ImageFormat::Png, which hides unsupported formats; update the
default branch to emit a warning for any unrecognized MIME before returning the
fallback. Keep the existing mapping for known image types, and make the warning
include the incoming mime value so callers can trace why decoding later failed.

In `@apps/desktop/src/video/audio.rs`:
- Line 37: The `extract_audio_from_mp4` function currently collapses all failure
cases into `Option`, which hides useful context for callers. Update
`extract_audio_from_mp4` and its internal error paths to return
`anyhow::Result<VideoAudio>` instead, and convert each `None` branch in the MP4
parse, audio track lookup, AAC frame extraction, probe, decoder creation, and
sample decoding flow into a descriptive error. Preserve the existing logging,
but make the returned errors distinguishable by attaching context at the
relevant failure points in `audio.rs`.
- Around line 168-187: The stereo-to-mono conversion in the audio decoding flow
is using the MP4 header channel count instead of the actual decoded stream
channel count. Update the logic around the decoded `SignalSpec`/`spec` and the
`mono_samples` conversion to base the `channels == 2` decision on the decoded
channel count, not `audio_track.channel_config()`. Keep the existing averaging
behavior in `all_samples` conversion, but ensure the channel count used there
reflects the real decoded audio so the `log::info!` message and output stay
correct when header and decoded channels differ.

In `@apps/desktop/src/video/player.rs`:
- Around line 185-191: The natural-completion path in `VideoPlayer::update()` is
clearing the completion sender before it can be used, so `on_complete()` never
receives its signal. Update the completion branch to take `self.completion_tx`
before calling `self.stop()`, then send on that captured sender after stopping,
using the `update()` and `stop()` methods to keep the flow correct and preserve
the public `on_complete()` behavior.

In `@apps/desktop/src/views/chat.rs`:
- Around line 112-119: Remove the unreachable fallback in chat view rendering:
`message_cache` in `selected_chat.map(|chat| app.get_message_list_cache(...))`
is guaranteed to be `Some` whenever `selected_chat` is present, so the
`unwrap_or_else` in `.when_some(selected_chat, ...)` is masking an invariant
violation. Update the logic around `when_some` and `MessageListCache` to either
use `unwrap()` with the existing guarantee or, preferably, destructure
`selected_chat` and `message_cache` together so the compiler enforces their
pairing and no empty-cache fallback can silently render zero messages.

In `@apps/desktop/src/views/pairing.rs`:
- Around line 12-31: Move the QR generation work out of the synchronous UI event
path by updating the `handle_event` flow in `AppState` to call `generate_qr_png`
inside `tokio::task::spawn_blocking` before storing the result. Keep
`generate_qr_png` as the QR/PNG encoder, but ensure the
`apps/desktop/src/app/mod.rs` event handler awaits the blocking task and only
then updates state so QR refreshes do not stall the desktop thread.
- Around line 61-66: The QR image rendering in the pairing view is recreating
the PNG buffer on every render via the `qr_code` branch in `pairing.rs`, which
needlessly clones bytes even though `cached.png_bytes` is already shared. Update
the `Image::from_bytes` / `img(ImageSource::Image(...))` path to reuse a cached
decoded `Image` or `Arc<Image>` instead of rebuilding it from `cached.png_bytes`
each time, keeping the logic localized around the `qr_code` handling in this
view.

In `@storages/chat-store/src/fts.rs`:
- Around line 18-43: ensure_fts currently creates the FTS table and triggers but
never backfills existing messages, so pre-existing rows stay unsearchable when
search is enabled later. Update ensure_fts to detect whether messages_fts was
newly created, and if so run an FTS5 rebuild after the CREATE VIRTUAL TABLE
step; keep the trigger creation logic unchanged and avoid rebuilding on later
calls.

In `@storages/chat-store/src/queries.rs`:
- Around line 36-46: The parse_jid helper in queries.rs logs the full raw JID on
parse failure, which can expose phone numbers in chat-store logs. Update the
warn! call inside parse_jid to avoid emitting the raw value and instead log a
redacted placeholder or generic message, while keeping the fallback to
Jid::default() unchanged.

In `@storages/chat-store/src/store.rs`:
- Around line 956-978: The unread count is being incremented twice for the same
logical message because both the placeholder path and the later message update
path call bump_chat with unread_delta. Update the MessageOp::Store /
insert_message flow so unread is only incremented when the row is newly
inserted, and avoid applying the unread_delta on in-place redelivery updates;
use the insert_message result (or a similar inserted/new-row signal) to decide
whether bump_chat should change unread_count.

In `@storages/chat-store/src/types.rs`:
- Around line 127-138: Add #[non_exhaustive] to the public StoreChange enum so
downstream pattern matches on StoreChange must include a fallback arm. Update
the enum definition in types.rs where StoreChange is declared, keeping its
existing variants (Chats, Messages, Contacts) unchanged and preserving its role
as the store invalidation signal.

In `@storages/sqlite-storage/src/shared.rs`:
- Around line 28-49: `SqliteStorage::run` currently maps
`spawn_blocking(...).await` `JoinError` into `StoreError::Database`, which hides
panic context from `f`. Update the `run` method to preserve debuggability by
handling the `JoinError` explicitly: either log the join failure before
converting it, or add a clear comment near the `.await` mapping in
`SqliteStorage::run` explaining that panics in `f` surface as
`StoreError::Database`. Keep the existing `acquire_owned`, `pool.get`, and
`spawn_blocking` flow unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d2211b02-5873-4d3c-a510-9390e29c5629

📥 Commits

Reviewing files that changed from the base of the PR and between d8fd43f and 8f70a14.

⛔ Files ignored due to path filters (6)
  • Cargo.lock is excluded by !**/*.lock
  • apps/desktop/Cargo.lock is excluded by !**/*.lock
  • apps/desktop/assets/icons/mic.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/pause.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/play.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/stop.svg is excluded by !**/*.svg
📒 Files selected for processing (63)
  • .github/workflows/desktop.yml
  • .gitignore
  • Cargo.toml
  • apps/desktop/Cargo.toml
  • apps/desktop/README.md
  • apps/desktop/src/app/calls.rs
  • apps/desktop/src/app/chats.rs
  • apps/desktop/src/app/media/mod.rs
  • apps/desktop/src/app/messages.rs
  • apps/desktop/src/app/mod.rs
  • apps/desktop/src/assets.rs
  • apps/desktop/src/audio/call_device.rs
  • apps/desktop/src/audio/encoder.rs
  • apps/desktop/src/audio/mod.rs
  • apps/desktop/src/audio/player.rs
  • apps/desktop/src/audio/recorder.rs
  • apps/desktop/src/audio/waveform.rs
  • apps/desktop/src/client/mod.rs
  • apps/desktop/src/client/whatsapp.rs
  • apps/desktop/src/components/avatar.rs
  • apps/desktop/src/components/call_popup.rs
  • apps/desktop/src/components/chat_header.rs
  • apps/desktop/src/components/chat_item.rs
  • apps/desktop/src/components/chat_list.rs
  • apps/desktop/src/components/input_area_view.rs
  • apps/desktop/src/components/message_bubble.rs
  • apps/desktop/src/components/message_list.rs
  • apps/desktop/src/components/mod.rs
  • apps/desktop/src/components/outgoing_call_popup.rs
  • apps/desktop/src/main.rs
  • apps/desktop/src/responsive.rs
  • apps/desktop/src/state/app_state.rs
  • apps/desktop/src/state/call.rs
  • apps/desktop/src/state/chat.rs
  • apps/desktop/src/state/events.rs
  • apps/desktop/src/state/mod.rs
  • apps/desktop/src/theme.rs
  • apps/desktop/src/utils.rs
  • apps/desktop/src/video/audio.rs
  • apps/desktop/src/video/mod.rs
  • apps/desktop/src/video/player.rs
  • apps/desktop/src/video/streaming.rs
  • apps/desktop/src/views/chat.rs
  • apps/desktop/src/views/error.rs
  • apps/desktop/src/views/loading.rs
  • apps/desktop/src/views/mod.rs
  • apps/desktop/src/views/pairing.rs
  • storages/chat-store/Cargo.toml
  • storages/chat-store/migrations/2026-07-09-000000_chat_store_initial/down.sql
  • storages/chat-store/migrations/2026-07-09-000000_chat_store_initial/up.sql
  • storages/chat-store/src/error.rs
  • storages/chat-store/src/fts.rs
  • storages/chat-store/src/lib.rs
  • storages/chat-store/src/materialize.rs
  • storages/chat-store/src/queries.rs
  • storages/chat-store/src/schema.rs
  • storages/chat-store/src/store.rs
  • storages/chat-store/src/types.rs
  • storages/chat-store/tests/chat_store_test.rs
  • storages/sqlite-storage/Cargo.toml
  • storages/sqlite-storage/src/lib.rs
  • storages/sqlite-storage/src/shared.rs
  • storages/sqlite-storage/src/sqlite_store.rs

Comment thread .github/workflows/desktop.yml
Comment thread apps/desktop/README.md Outdated
Comment thread apps/desktop/src/app/calls.rs
Comment thread apps/desktop/src/app/messages.rs Outdated
Comment thread apps/desktop/src/app/mod.rs
Comment thread storages/chat-store/src/fts.rs
Comment thread storages/chat-store/src/queries.rs
Comment thread storages/chat-store/src/store.rs
Comment thread storages/chat-store/src/types.rs
Comment thread storages/sqlite-storage/src/shared.rs
@coderabbitai coderabbitai Bot removed the size-increase-ok Accepted binary-size increase: downgrades the per-PR size gate to a warning label Jul 9, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a4c419e5a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/desktop/src/client/whatsapp.rs Outdated
Comment thread storages/chat-store/src/store.rs
Comment thread storages/chat-store/src/store.rs
Comment thread apps/desktop/src/audio/recorder.rs Outdated
Comment thread apps/desktop/src/audio/player.rs Outdated
Comment thread apps/desktop/src/app/mod.rs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48b8567a54

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread storages/chat-store/src/store.rs Outdated
Comment thread storages/chat-store/src/store.rs
Comment thread apps/desktop/src/client/whatsapp.rs Outdated
Comment thread apps/desktop/src/app/mod.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

♻️ Duplicate comments (5)
apps/desktop/src/components/avatar.rs (1)

21-26: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Consolidate duplicate avatar constructors — still unaddressed.

from_initial still duplicates render instead of delegating. This was flagged before and hasn't been fixed. One line change prevents future drift.

♻️ Proposed refactor
 pub fn from_initial(initial: char, size: f32) -> impl IntoElement {
-    GpuiAvatar::new()
-        .name(initial.to_string())
-        .with_size(gpui_component::Size::Large)
-        .size(gpui::px(size))
+    Self::render(initial.to_string(), size)
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/components/avatar.rs` around lines 21 - 26, The avatar
constructor still duplicates the same builder chain as render instead of
delegating. Update from_initial in Avatar to call into render (or the shared
construction path) using the provided initial converted to a string and the same
size handling, so both methods stay aligned and future changes only need to be
made in one place.
apps/desktop/src/video/audio.rs (2)

168-187: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Stereo-to-mono conversion still uses MP4 header channel count, not decoded stream count — unchanged.

channels at line 168 comes from audio_track.channel_config() (the container header), but the actual decoded channel count is in spec from decoded.spec() at line 148. If header and stream disagree, the mono conversion will silently corrupt audio — averaging pairs when there's one channel, or not averaging when there are two. This was flagged before and hasn't been fixed.

🔧 Proposed fix
     let mut all_samples: Vec<f32> = Vec::new();
+    let mut actual_channels: u8 = channels;
 
     // Decode all audio packets
     while let Ok(packet) = format.next_packet() {
         if packet.track_id() != adts_track_id {
             continue;
         }
 
         match decoder.decode(&packet) {
             Ok(decoded) => {
                 let spec = *decoded.spec();
+                actual_channels = spec.channels.count() as u8;
                 let duration = decoded.capacity() as u64;
 
                 let mut sample_buf = SampleBuffer::<f32>::new(duration, spec);
                 sample_buf.copy_interleaved_ref(decoded);
 
                 all_samples.extend_from_slice(sample_buf.samples());
             }
             Err(e) => {
                 log::debug!("Audio decode error (skipping frame): {}", e);
             }
         }
     }
@@ -168,7 +169,7 @@
     // Convert stereo to mono if needed (average L and R channels)
-    let mono_samples = if channels == 2 {
+    let mono_samples = if actual_channels == 2 {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/video/audio.rs` around lines 168 - 187, The mono conversion
in audio processing still relies on the container/header channel count instead
of the decoded stream’s actual channel count. Update the logic around the
stereo-to-mono branch in audio.rs to use the channel info from decoded.spec()
(or a value derived from it) rather than audio_track.channel_config(), and
ensure the conversion path in the mono_samples block only averages when the
decoded audio is truly stereo.

37-37: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

extract_audio_from_mp4 still returns Option instead of anyhow::Result — guideline violation persists.

This function has six distinct failure paths (MP4 parse, no audio track, no AAC frames, probe failure, decoder creation, no samples decoded) all collapsed into None. Per coding guidelines, multi-failure functions should use anyhow. Callers can't distinguish recoverable vs fatal errors.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/video/audio.rs` at line 37, `extract_audio_from_mp4` is
still collapsing multiple distinct failure paths into `Option`; update it to
return `anyhow::Result<VideoAudio>` and propagate each failure case with
meaningful context instead of `None`. Adjust the full flow inside
`extract_audio_from_mp4` to use `?` for MP4 parsing, track/frame selection,
probe/decoder creation, and decoding, and then update any callers of
`extract_audio_from_mp4` to handle `Result` rather than checking for `Option`.

Source: Coding guidelines

apps/desktop/src/views/pairing.rs (2)

12-31: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

QR generation still not wrapped in spawn_blocking — coding guideline violation persists.

generate_qr_png does CPU-heavy QR encoding and PNG compression synchronously. Per coding guidelines, all heavy CPU work must be wrapped in tokio::task::spawn_blocking. The call site in app/mod.rs was flagged before and hasn't been addressed. This will stall the UI thread on every QR refresh.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/views/pairing.rs` around lines 12 - 31, The QR generation
work in generate_qr_png is still running synchronously and must be moved off the
UI thread. Wrap the CPU-heavy QR code creation and PNG encoding in
tokio::task::spawn_blocking, and update the call site that triggers it so the
result is awaited asynchronously instead of calling generate_qr_png directly.
Keep the logic centered around generate_qr_png and the app/mod.rs QR refresh
path.

Source: Coding guidelines


61-66: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

QR PNG buffer still cloned on every render — unchanged from prior review.

cached.png_bytes.as_ref().clone() rebuilds a Vec<u8> from the Arc<Vec<u8>> on every frame. Cache the decoded Image/Arc<Image> instead of re-decoding from bytes each time.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/views/pairing.rs` around lines 61 - 66, The QR image
creation in the pairing view is still rebuilding from `cached.png_bytes` on
every render. Update the `pairing.rs` QR rendering path to cache the decoded
`Image` or `Arc<Image>` alongside the QR data, and have the widget use that
cached image instead of calling `Image::from_bytes` with
`cached.png_bytes.as_ref().clone()` each frame. Keep the change localized to the
QR display logic so the `img(ImageSource::Image(...))` path reuses the
pre-decoded asset.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/README.md`:
- Around line 15-47: Update the desktop README architecture tree to include the
missing modules and components currently present in the workspace, especially
the audio/, video/, responsive.rs, utils.rs, assets.rs, and call popup component
files so the layout matches the real codebase. In the Features Status section,
mark the call UI and QR code rendering items as completed since the relevant
components and CachedQrCode already exist. In Future Improvements, remove or
rewrite items that are no longer pending, especially message sending status and
WebRTC call integration, to reflect the current implemented behavior in the
desktop app.
- Line 28: The call.rs module description is outdated and references a
non-existent ActiveCall type. Update the README entry for call.rs to match the
actual exports from state/mod.rs by listing the real symbols (CallId,
IncomingCall, OutgoingCall, OutgoingCallState) and removing ActiveCall so the
module map stays accurate.

In `@apps/desktop/src/app/messages.rs`:
- Around line 44-47: The virtual list item sizing in MessageListCache::new()
still uses a fixed px(600.) width, so update the size calculation to derive the
width from ResponsiveLayout instead of hardcoding it. Locate the sizing logic in
apps/desktop/src/app/messages.rs around calculate_message_height(msg,
show_sender, max_media_size), and thread the responsive message-list width into
the virtual item dimensions so list layout stays aligned with the current
viewport.

In `@apps/desktop/src/audio/encoder.rs`:
- Around line 129-146: EncoderError currently uses manual Display and
std::error::Error impls, which should be replaced with thiserror-derived typed
errors. Add thiserror to apps/desktop/Cargo.toml, then update EncoderError in
encoder.rs to derive the appropriate error traits and annotate each variant with
the right error message/source behavior so the enum no longer needs hand-written
fmt::Display or Error impls.

In `@apps/desktop/src/audio/player.rs`:
- Around line 287-310: PlayerError is still using manual Display and Error impls
instead of the required thiserror-based typed error pattern. Update the
PlayerError enum in player.rs to derive the appropriate thiserror traits, remove
the hand-written std::fmt::Display and std::error::Error impl blocks, and make
sure the variant messages are preserved via thiserror attributes; keep the
approach consistent with the EncoderError fix and verify the thiserror
dependency is available in the desktop crate.

In `@apps/desktop/src/audio/recorder.rs`:
- Around line 27-44: The resampling logic in Recorder::resample_to_16khz is
using nearest-neighbor sampling by selecting every Nth sample, which hurts audio
quality. Update this method to perform linear interpolation between adjacent
samples when mapping from the original sample rate to TARGET_SAMPLE_RATE,
keeping the existing early return for already-16kHz input and preserving the
current output length calculation.
- Around line 96-107: The config selection in init() is too restrictive because
it only considers mono configs, which can cause NoSupportedConfig even though
the stream callback in Recorder has a multi-channel downmix path. Update the
supported-config search to first prefer mono configs for CAPTURE_SAMPLE_RATE,
but also fall back to stereo/multi-channel configs when no mono option exists,
using the existing downmix handling in the callback. Keep the logic localized
around init() and the supported config loop so the multi-channel path becomes
reachable.
- Around line 219-244: Replace the manual error boilerplate for RecorderError
with a thiserror-based derive so the enum itself defines the messages. Update
RecorderError to use the thiserror derive and move each variant’s text into its
attributes, then remove the custom std::fmt::Display implementation and the
empty std::error::Error impl. Keep the enum variants and their meanings
unchanged, and use RecorderError as the unique symbol to locate this refactor.
- Around line 147-159: The recording callback in recorder::Recorder is appending
samples to the shared buffer without any upper bound, so add a max-duration or
max-sample-count guard inside the callback before pushing to samples. Use the
existing capture path around the cpal input callback and the samples lock to
stop extending the Vec<f32> once the limit is reached, and make sure the
recording state is marked complete/ignored so input is no longer accumulated.

In `@apps/desktop/src/client/whatsapp.rs`:
- Around line 389-416: The reaction-handling block in `handle_message` still
mixes an outer `if let` with an inner let-chain, which is inconsistent with the
file’s broader let-chain style. Refactor the
`base_msg.reaction_message.as_option()` handling to use a `let Some(reaction) =
... else { ... };`-style early-exit structure, keeping the existing `return`
behavior intact while preserving the `key.id` and `key.remote_jid` checks inside
the reaction path.

In `@apps/desktop/src/video/streaming.rs`:
- Around line 411-422: The reset_decoder method currently ignores Decoder::new()
failures, leaving the old decoder and last_decoded_index state in place. Update
reset_decoder to handle the Err case explicitly: log the failure with context
from Decoder::new(), and ensure the decoder state is not left stale (including
resetting last_decoded_index as part of the reset path). Keep the SPS/PPS
reinitialization in the successful new_decoder path using self.sps_pps.

---

Duplicate comments:
In `@apps/desktop/src/components/avatar.rs`:
- Around line 21-26: The avatar constructor still duplicates the same builder
chain as render instead of delegating. Update from_initial in Avatar to call
into render (or the shared construction path) using the provided initial
converted to a string and the same size handling, so both methods stay aligned
and future changes only need to be made in one place.

In `@apps/desktop/src/video/audio.rs`:
- Around line 168-187: The mono conversion in audio processing still relies on
the container/header channel count instead of the decoded stream’s actual
channel count. Update the logic around the stereo-to-mono branch in audio.rs to
use the channel info from decoded.spec() (or a value derived from it) rather
than audio_track.channel_config(), and ensure the conversion path in the
mono_samples block only averages when the decoded audio is truly stereo.
- Line 37: `extract_audio_from_mp4` is still collapsing multiple distinct
failure paths into `Option`; update it to return `anyhow::Result<VideoAudio>`
and propagate each failure case with meaningful context instead of `None`.
Adjust the full flow inside `extract_audio_from_mp4` to use `?` for MP4 parsing,
track/frame selection, probe/decoder creation, and decoding, and then update any
callers of `extract_audio_from_mp4` to handle `Result` rather than checking for
`Option`.

In `@apps/desktop/src/views/pairing.rs`:
- Around line 12-31: The QR generation work in generate_qr_png is still running
synchronously and must be moved off the UI thread. Wrap the CPU-heavy QR code
creation and PNG encoding in tokio::task::spawn_blocking, and update the call
site that triggers it so the result is awaited asynchronously instead of calling
generate_qr_png directly. Keep the logic centered around generate_qr_png and the
app/mod.rs QR refresh path.
- Around line 61-66: The QR image creation in the pairing view is still
rebuilding from `cached.png_bytes` on every render. Update the `pairing.rs` QR
rendering path to cache the decoded `Image` or `Arc<Image>` alongside the QR
data, and have the widget use that cached image instead of calling
`Image::from_bytes` with `cached.png_bytes.as_ref().clone()` each frame. Keep
the change localized to the QR display logic so the
`img(ImageSource::Image(...))` path reuses the pre-decoded asset.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9ab34688-94b1-427b-8eb2-317b40fe84ec

📥 Commits

Reviewing files that changed from the base of the PR and between 8f70a14 and 3a4c419.

⛔ Files ignored due to path filters (5)
  • apps/desktop/Cargo.lock is excluded by !**/*.lock
  • apps/desktop/assets/icons/mic.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/pause.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/play.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/stop.svg is excluded by !**/*.svg
📒 Files selected for processing (54)
  • .github/workflows/desktop.yml
  • .gitignore
  • Cargo.toml
  • apps/desktop/Cargo.toml
  • apps/desktop/README.md
  • apps/desktop/src/app/calls.rs
  • apps/desktop/src/app/chats.rs
  • apps/desktop/src/app/media/mod.rs
  • apps/desktop/src/app/messages.rs
  • apps/desktop/src/app/mod.rs
  • apps/desktop/src/assets.rs
  • apps/desktop/src/audio/call_device.rs
  • apps/desktop/src/audio/encoder.rs
  • apps/desktop/src/audio/mod.rs
  • apps/desktop/src/audio/player.rs
  • apps/desktop/src/audio/recorder.rs
  • apps/desktop/src/audio/waveform.rs
  • apps/desktop/src/client/mod.rs
  • apps/desktop/src/client/whatsapp.rs
  • apps/desktop/src/components/avatar.rs
  • apps/desktop/src/components/call_popup.rs
  • apps/desktop/src/components/chat_header.rs
  • apps/desktop/src/components/chat_item.rs
  • apps/desktop/src/components/chat_list.rs
  • apps/desktop/src/components/input_area_view.rs
  • apps/desktop/src/components/message_bubble.rs
  • apps/desktop/src/components/message_list.rs
  • apps/desktop/src/components/mod.rs
  • apps/desktop/src/components/outgoing_call_popup.rs
  • apps/desktop/src/main.rs
  • apps/desktop/src/responsive.rs
  • apps/desktop/src/state/app_state.rs
  • apps/desktop/src/state/call.rs
  • apps/desktop/src/state/chat.rs
  • apps/desktop/src/state/events.rs
  • apps/desktop/src/state/mod.rs
  • apps/desktop/src/theme.rs
  • apps/desktop/src/utils.rs
  • apps/desktop/src/video/audio.rs
  • apps/desktop/src/video/mod.rs
  • apps/desktop/src/video/player.rs
  • apps/desktop/src/video/streaming.rs
  • apps/desktop/src/views/chat.rs
  • apps/desktop/src/views/error.rs
  • apps/desktop/src/views/loading.rs
  • apps/desktop/src/views/mod.rs
  • apps/desktop/src/views/pairing.rs
  • storages/chat-store/src/error.rs
  • storages/chat-store/src/fts.rs
  • storages/chat-store/src/materialize.rs
  • storages/chat-store/src/queries.rs
  • storages/chat-store/src/store.rs
  • storages/chat-store/src/types.rs
  • storages/chat-store/tests/chat_store_test.rs

Comment thread apps/desktop/README.md Outdated
Comment thread apps/desktop/README.md Outdated
Comment thread apps/desktop/src/app/messages.rs
Comment thread apps/desktop/src/audio/encoder.rs
Comment thread apps/desktop/src/audio/player.rs
Comment thread apps/desktop/src/audio/recorder.rs
Comment thread apps/desktop/src/audio/recorder.rs Outdated
Comment thread apps/desktop/src/audio/recorder.rs
Comment thread apps/desktop/src/client/whatsapp.rs
Comment thread apps/desktop/src/video/streaming.rs
@jlucaso1
jlucaso1 force-pushed the feat-call-ui branch 2 times, most recently from f81fab4 to 04a19e7 Compare July 9, 2026 19:01
Both send paths returned silently when the chat JID failed to parse,
leaving the optimistic bubble stuck without an error indicator; emit
SendFailed for the local id like the other error arms.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbVittC45t2xPuCqku1GCb
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 4 files (changes from recent commits).

Requires human review: This PR adds a full desktop app with large-scale, multi-subsystem changes. AI review found no issues, but it requires human review to assess architecture and potential regressions.

Re-trigger cubic

Comment thread apps/desktop/src/app/mod.rs Outdated
load_history caps at HISTORY_CHAT_LIMIT, so absence from a truncated
load can just mean the chat fell past the window — pruning against it
silently dropped visible chats for accounts with more than the limit.
HistoryLoaded now carries a completeness flag (load came back under the
limit) and the prune runs only then; archive/delete-elsewhere removal
still works for complete loads.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbVittC45t2xPuCqku1GCb
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread apps/desktop/src/state/events.rs
PN/LID collapsing can shrink a truncated 100-entry fetch back under the
limit, which would mark it complete and let the prune drop chats that
only fell past the window; load_history now returns the flag computed
from the entry count before dedup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbVittC45t2xPuCqku1GCb
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 21 files (changes from recent commits).

You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread apps/desktop/src/main.rs
Comment thread apps/desktop/src/client/whatsapp.rs Outdated
Comment thread wacore/src/history_sync.rs Outdated
Comment thread apps/desktop/src/state/chat.rs
Comment thread apps/desktop/src/state/chat.rs Outdated
Comment thread storages/chat-store/src/store.rs Outdated
Comment thread apps/desktop/src/audio/call_device.rs
Comment thread apps/desktop/src/client/whatsapp.rs
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 8 files (changes from recent commits).

You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Auto-approved: Adds a new desktop app in a standalone workspace, consuming the library via path deps. No existing library code is modified. The change is bounded to new files and clearly beneficial as a consumer test and feature addition.

Re-trigger cubic

claude added 2 commits July 14, 2026 14:01
A freshly paired device has an empty store while live chats already
exist in memory; any early StoreChange (e.g. a push-name commit) made
the reloader emit an empty complete load and the prune wiped them.
Chats now carry store provenance: only rows a load has returned before
are pruned when a complete load stops returning them, so pairing-window
live chats survive while delete/archive-elsewhere removal keeps working.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbVittC45t2xPuCqku1GCb
The comment claimed per-conversation extraction only ran when the bulk
block was absent; both sources are harvested.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbVittC45t2xPuCqku1GCb
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 4 files (changes from recent commits).

You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Requires human review: This PR adds a new desktop application with a novel UI framework (GPUI) and durable chat storage, which is a significant expansion of scope requiring human review of architectural, dependency, and data-handling tradeoffs.

Re-trigger cubic

@jlucaso1

Copy link
Copy Markdown
Collaborator Author

this branch is usable? I don't want to use the web client and I just need the chat.

Now is usable, feel free to try

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/README.md`:
- Line 46: Update the call.rs entry in the desktop README to remove the
nonexistent ActiveCall type and document only the actual call structs, including
IncomingCall and OutgoingCall.

In `@apps/desktop/src/app/mod.rs`:
- Around line 1804-1810: In both video-completion cleanup branches within the
media update flow, clear app.audio_owner alongside app.audio_player.stop().
Update both locations identified by the duplicated completion handling,
preserving the existing active_media, video_update_task, and notification
cleanup.
- Around line 1882-1908: When complete hydration in the chat-loading flow
removes store-backed chats via the retain operation, also invalidate each
removed chat’s message cache before dropping it. Update the logic around the
existing chats.retain call and invalidate by JID only for chats excluded from
retention, while preserving caches for retained and newly merged chats.

In `@apps/desktop/src/audio/call_device.rs`:
- Around line 20-45: Replace the linear interpolation in resample_into with a
stateful band-limited resampling approach, such as a low-pass FIR filter
followed by decimation, preserving filter history across calls and using the
existing fractional cursor for block continuity. Ensure the common 48→16 kHz
path attenuates frequencies above the 8 kHz Nyquist limit before downsampling.
- Around line 241-244: Update the readiness barriers in the audio setup flow,
including the nearby second recv call around the corresponding mic/speaker
startup path, to use recv_timeout with an appropriate startup timeout. Convert
timeout or channel-closure failures into a descriptive setup error while
preserving the existing handling of successful readiness signals.
- Around line 263-264: Update the CPAL error callbacks near the device stream
setup to propagate stream failures into the call shutdown path instead of only
logging them: the microphone callback must clear the shared alive state, and the
speaker callback must set teardown and close the drain channel. Preserve the
existing error logging while wiring both callbacks to the corresponding shared
state used by the call lifecycle.

In `@apps/desktop/src/audio/player.rs`:
- Around line 270-299: Update the packet handling around the OpusHead/OpusTags
checks so packets are skipped only when their payload has the corresponding
header signature, rather than based on packet_count. Preserve decoder
initialization for streams with OpusHead, and initialize the fallback decoder
before decoding the first non-header packet when headers are absent.
- Around line 139-180: Update AudioPlayer::play_samples, pause, resume, and the
stream error callback so is_playing changes only after successful CPAL
operations. Move startup activation until build_stream and stream.play succeed,
restore the prior state and resolve the completion channel on startup failure,
propagate pause/resume errors without flipping state, and have the error
callback clear playback state and complete the pending completion receiver.

In `@apps/desktop/src/audio/recorder.rs`:
- Around line 145-169: Update the scoring logic in the supported-configuration
selection loop so capture-rate support ranks ahead of sample format: a
configuration supporting CAPTURE_SAMPLE_RATE must outrank lower-rate F32
configurations, while retaining the existing mono preference and candidate
construction behavior.

In `@apps/desktop/src/audio/waveform.rs`:
- Around line 13-33: Update the waveform RMS bucket generation using
WAVEFORM_SAMPLES so samples are distributed across all waveform buckets rather
than relying on fixed-size chunks that can produce too few values; ensure a
65-sample input populates the final bucket when it has signal. Add a regression
test covering 65 samples and asserting the final waveform value is non-zero,
while preserving normalization and padding behavior for other inputs.

In `@apps/desktop/src/components/call_popup.rs`:
- Around line 119-143: The call action container in the popup currently uses a
pointer-only div; replace or augment it with a focusable keyboard-operable
control around the existing on_click behavior. Ensure Enter and Space activate
the same callback for accepting or declining calls, while preserving the current
styling and visual label.

In `@apps/desktop/src/components/message_bubble.rs`:
- Around line 390-400: The render_image_from_bytes function currently clones
image bytes and decodes the image on every render. Update this path to obtain
and reuse a cached Arc<Image>, following the existing sticker-preview caching
approach, and render the cached decoded image directly while preserving the
current format and sizing behavior.

In `@apps/desktop/src/components/message_list.rs`:
- Around line 16-23: Remove the unused _playing_message_id parameter from
render_message_list and update every call site to stop passing it, while
preserving the closure’s existing use of app.playing_message_id() for each
render.

In `@apps/desktop/src/video/streaming.rs`:
- Around line 129-131: Validate the dimensions obtained in the video streaming
path before the RGBA buffer allocation: reject zero dimensions and pixel counts
above the supported maximum, then use checked multiplication when computing the
buffer size. Update the allocation logic around the video dimension handling and
the later RGBA buffer creation so invalid or overflowing sizes return an
appropriate error instead of allocating.
- Around line 27-28: Update avcc_to_annexb to accept the track’s avcC
lengthSizeMinusOne value and derive the NAL prefix width as 1, 2, or 4 bytes
instead of using NAL_LENGTH_SIZE. Read and pass this value from the video track
metadata, validate malformed avcC records and NAL lengths, and reject
unsupported or inconsistent widths before parsing samples.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dd8cc6db-e945-4772-8ccb-edfb5a7a7989

📥 Commits

Reviewing files that changed from the base of the PR and between fcd7bc1 and 7ac4c59.

⛔ Files ignored due to path filters (5)
  • apps/desktop/Cargo.lock is excluded by !**/*.lock
  • apps/desktop/assets/icons/mic.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/pause.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/play.svg is excluded by !**/*.svg
  • apps/desktop/assets/icons/stop.svg is excluded by !**/*.svg
📒 Files selected for processing (54)
  • .github/workflows/desktop.yml
  • .gitignore
  • Cargo.toml
  • apps/desktop/Cargo.toml
  • apps/desktop/README.md
  • apps/desktop/src/app/calls.rs
  • apps/desktop/src/app/chats.rs
  • apps/desktop/src/app/media/mod.rs
  • apps/desktop/src/app/messages.rs
  • apps/desktop/src/app/mod.rs
  • apps/desktop/src/assets.rs
  • apps/desktop/src/audio/call_device.rs
  • apps/desktop/src/audio/encoder.rs
  • apps/desktop/src/audio/mod.rs
  • apps/desktop/src/audio/player.rs
  • apps/desktop/src/audio/recorder.rs
  • apps/desktop/src/audio/waveform.rs
  • apps/desktop/src/client/mod.rs
  • apps/desktop/src/client/whatsapp.rs
  • apps/desktop/src/components/avatar.rs
  • apps/desktop/src/components/call_popup.rs
  • apps/desktop/src/components/chat_header.rs
  • apps/desktop/src/components/chat_item.rs
  • apps/desktop/src/components/chat_list.rs
  • apps/desktop/src/components/input_area_view.rs
  • apps/desktop/src/components/message_bubble.rs
  • apps/desktop/src/components/message_list.rs
  • apps/desktop/src/components/mod.rs
  • apps/desktop/src/components/outgoing_call_popup.rs
  • apps/desktop/src/main.rs
  • apps/desktop/src/responsive.rs
  • apps/desktop/src/state/app_state.rs
  • apps/desktop/src/state/call.rs
  • apps/desktop/src/state/chat.rs
  • apps/desktop/src/state/events.rs
  • apps/desktop/src/state/mod.rs
  • apps/desktop/src/theme.rs
  • apps/desktop/src/utils.rs
  • apps/desktop/src/video/audio.rs
  • apps/desktop/src/video/mod.rs
  • apps/desktop/src/video/player.rs
  • apps/desktop/src/video/streaming.rs
  • apps/desktop/src/views/chat.rs
  • apps/desktop/src/views/error.rs
  • apps/desktop/src/views/loading.rs
  • apps/desktop/src/views/mod.rs
  • apps/desktop/src/views/pairing.rs
  • src/client/accessors.rs
  • src/client/app_state.rs
  • src/history_sync.rs
  • src/pair.rs
  • storages/chat-store/src/store.rs
  • storages/chat-store/tests/chat_store_test.rs
  • wacore/src/history_sync.rs

Comment thread apps/desktop/README.md
│ │ ├── mod.rs
│ │ ├── app_state.rs # AppState enum (Loading, Connected, etc.)
│ │ ├── chat.rs # Chat, ChatMessage, MediaContent structs
│ │ ├── call.rs # IncomingCall, OutgoingCall, ActiveCall structs

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

ActiveCall still doesn't exist — this line was flagged before and only half-fixed.

OutgoingCall got added, but ActiveCall is still listed and it's not a real type anywhere in state/mod.rs. If we're going to fix the doc, let's actually fix it — half a fix isn't a fix.

📝 Fix the call.rs description
-│   │   ├── call.rs          # IncomingCall, OutgoingCall, ActiveCall structs
+│   │   ├── call.rs          # CallId, IncomingCall, OutgoingCall, OutgoingCallState structs
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/README.md` at line 46, Update the call.rs entry in the desktop
README to remove the nonexistent ActiveCall type and document only the actual
call structs, including IncomingCall and OutgoingCall.

Comment on lines +1804 to +1810
Ok(()) => {
// Video completed naturally
let _ = entity.update(cx, |app, cx| {
app.active_media = ActiveMedia::None;
app.video_update_task = None;
app.audio_player.stop();
cx.notify();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Clear audio_owner whenever video completion stops the sink.

Both exits stop audio_player but leave ownership pointing at the finished video. The resume path treats that stale value as proof that audio can be resumed, potentially replaying the video silently.

Proposed fix
 app.audio_player.stop();
+app.audio_owner = None;

Apply this beside both audio_player.stop() calls.

Also applies to: 1845-1850

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/app/mod.rs` around lines 1804 - 1810, In both
video-completion cleanup branches within the media update flow, clear
app.audio_owner alongside app.audio_player.stop(). Update both locations
identified by the duplicated completion handling, preserving the existing
active_media, video_update_task, and notification cleanup.

Comment on lines +1882 to +1908
if complete {
let loaded: std::collections::HashSet<&str> =
chats.iter().map(|c| c.jid.as_str()).collect();
self.chats.retain(|c| {
!c.from_store
|| loaded.contains(c.jid.as_str())
|| self.selected_chat.as_deref() == Some(c.jid.as_str())
});
}
for chat in chats {
// Later loads (post-HistorySync re-hydration) fold into
// chats the UI already shows instead of being dropped.
match self.chats.iter_mut().find(|c| c.jid == chat.jid) {
Some(existing) => {
let jid = chat.jid.clone();
existing.merge_history(chat);
// The open chat was read locally the moment the
// message arrived; the store row commits with the
// unread bump before our receipt lands, so the
// hydrated counter must not resurrect the badge.
if self.selected_chat.as_deref() == Some(jid.as_str()) {
existing.mark_as_read();
}
self.invalidate_message_cache(&jid);
}
None => self.chats.push(chat),
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Evict message caches when complete hydration removes chats.

The retain operation drops chats but preserves their MessageListCache. If that JID is later recreated with the same message count and layout inputs, get_message_list_cache can render the removed chat’s messages. This data cannot be allowed to cross chat lifetimes.

Proposed fix
 if complete {
     let loaded: std::collections::HashSet<&str> =
         chats.iter().map(|c| c.jid.as_str()).collect();
     self.chats.retain(|c| {
         !c.from_store
             || loaded.contains(c.jid.as_str())
             || self.selected_chat.as_deref() == Some(c.jid.as_str())
     });
+
+    let retained: std::collections::HashSet<&str> =
+        self.chats.iter().map(|chat| chat.jid.as_str()).collect();
+    self.message_list_cache
+        .borrow_mut()
+        .retain(|jid, _| retained.contains(jid.as_str()));
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if complete {
let loaded: std::collections::HashSet<&str> =
chats.iter().map(|c| c.jid.as_str()).collect();
self.chats.retain(|c| {
!c.from_store
|| loaded.contains(c.jid.as_str())
|| self.selected_chat.as_deref() == Some(c.jid.as_str())
});
}
for chat in chats {
// Later loads (post-HistorySync re-hydration) fold into
// chats the UI already shows instead of being dropped.
match self.chats.iter_mut().find(|c| c.jid == chat.jid) {
Some(existing) => {
let jid = chat.jid.clone();
existing.merge_history(chat);
// The open chat was read locally the moment the
// message arrived; the store row commits with the
// unread bump before our receipt lands, so the
// hydrated counter must not resurrect the badge.
if self.selected_chat.as_deref() == Some(jid.as_str()) {
existing.mark_as_read();
}
self.invalidate_message_cache(&jid);
}
None => self.chats.push(chat),
}
if complete {
let loaded: std::collections::HashSet<&str> =
chats.iter().map(|c| c.jid.as_str()).collect();
self.chats.retain(|c| {
!c.from_store
|| loaded.contains(c.jid.as_str())
|| self.selected_chat.as_deref() == Some(c.jid.as_str())
});
let retained: std::collections::HashSet<&str> =
self.chats.iter().map(|chat| chat.jid.as_str()).collect();
self.message_list_cache
.borrow_mut()
.retain(|jid, _| retained.contains(jid.as_str()));
}
for chat in chats {
// Later loads (post-HistorySync re-hydration) fold into
// chats the UI already shows instead of being dropped.
match self.chats.iter_mut().find(|c| c.jid == chat.jid) {
Some(existing) => {
let jid = chat.jid.clone();
existing.merge_history(chat);
// The open chat was read locally the moment the
// message arrived; the store row commits with the
// unread bump before our receipt lands, so the
// hydrated counter must not resurrect the badge.
if self.selected_chat.as_deref() == Some(jid.as_str()) {
existing.mark_as_read();
}
self.invalidate_message_cache(&jid);
}
None => self.chats.push(chat),
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/app/mod.rs` around lines 1882 - 1908, When complete
hydration in the chat-loading flow removes store-backed chats via the retain
operation, also invalidate each removed chat’s message cache before dropping it.
Update the logic around the existing chats.retain call and invalidate by JID
only for chats excluded from retention, while preserving caches for retained and
newly merged chats.

Comment on lines +20 to +45
/// Linear resample `src` (at `src_rate`) into the 16 kHz `out`, carrying a fractional read cursor
/// across calls so block boundaries don't click. Allocation-free: writes into the caller's `out`.
fn resample_into(src: &[i16], src_rate: u32, dst_rate: u32, pos: &mut f64, out: &mut Vec<i16>) {
if src.is_empty() {
return;
}
let step = src_rate as f64 / dst_rate as f64;
// `pos` is the fractional index into a virtual stream; it started somewhere in the previous
// block's tail, so rebase it into this block.
let mut p = *pos;
while p < src.len() as f64 {
let i = p as usize;
let frac = p - i as f64;
let a = src[i] as f64;
let b = if i + 1 < src.len() {
src[i + 1] as f64
} else {
a
};
out.push((a + (b - a) * frac).round() as i16);
p += step;
}
// Carry the leftover fraction (relative to the next block's start) so the next call continues
// smoothly instead of restarting at 0.
*pos = p - src.len() as f64;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not decimate microphone audio without a low-pass filter.

For the common 48→16 kHz path, this effectively takes every third sample. Content above 8 kHz aliases into the voice band. Use a stateful band-limited resampler or FIR before decimation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/audio/call_device.rs` around lines 20 - 45, Replace the
linear interpolation in resample_into with a stateful band-limited resampling
approach, such as a low-pass FIR filter followed by decimation, preserving
filter history across calls and using the existing fractional cursor for block
continuity. Ensure the common 48→16 kHz path attenuates frequencies above the 8
kHz Nyquist limit before downsampling.

Comment on lines +241 to +244
// Build/play are near-instant; a dead sender means the thread panicked.
ready_rx
.recv()
.map_err(|_| anyhow!("mic stream thread exited before signaling readiness"))??;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound both audio startup barriers.

These recv() calls wait forever if device initialization wedges. Use recv_timeout and return a setup error so call setup cannot hang indefinitely.

Also applies to: 419-422

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/audio/call_device.rs` around lines 241 - 244, Update the
readiness barriers in the audio setup flow, including the nearby second recv
call around the corresponding mic/speaker startup path, to use recv_timeout with
an appropriate startup timeout. Convert timeout or channel-closure failures into
a descriptive setup error while preserving the existing handling of successful
readiness signals.

Comment on lines +119 to +143
div()
.id(SharedString::from(format!("call-btn-{}", label)))
.w(px(56.0))
.h(px(56.0))
.bg(rgb(color))
.rounded_full()
.flex()
.items_center()
.justify_center()
.cursor_pointer()
.hover(|s| s.opacity(0.8))
.on_click(on_click)
.child(
div()
.text_color(rgb(colors::WHITE))
.text_lg()
.font_weight(gpui::FontWeight::BOLD)
.child(icon),
),
)
.child(
div()
.text_xs()
.text_color(rgb(colors::TEXT_SECONDARY))
.child(label),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep outline apps/desktop/src/components/call_popup.rs --items all --view expanded
rg -n -C3 'accept_call|decline_call|KeyBinding::new|on_key_down|focusable|tab_index' apps/desktop/src

Repository: oxidezap/whatsapp-rust

Length of output: 5113


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== call_popup.rs =="
cat -n apps/desktop/src/components/call_popup.rs | sed -n '1,220p'

echo
echo "== keybinding/search =="
rg -n -C2 'KeyBinding::new|bind_keys|on_key_down|focusable|tab_index|button\(|div\(\).*on_click|accept_call|decline_call' apps/desktop/src

Repository: oxidezap/whatsapp-rust

Length of output: 10951


Make call actions keyboard-operable. These are pointer-only divs, so accepting or declining a call still requires a mouse. Use a focusable control with Enter/Space handling, or add equivalent global bindings for the popup.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/components/call_popup.rs` around lines 119 - 143, The call
action container in the popup currently uses a pointer-only div; replace or
augment it with a focusable keyboard-operable control around the existing
on_click behavior. Ensure Enter and Space activate the same callback for
accepting or declining calls, while preserving the current styling and visual
label.

Comment on lines +390 to +400
fn render_image_from_bytes(
data: Arc<Vec<u8>>,
mime_type: &str,
width: f32,
height: f32,
rounded: bool,
) -> gpui::Img {
let format = mime_to_image_format(mime_type);
let image_data = Arc::unwrap_or_clone(data);
let image = Image::from_bytes(format, image_data);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep outline apps/desktop/src/components/message_bubble.rs \
  --match 'render_message_bubble|render_image_from_bytes' --view expanded

rg -n -C4 --type=rust \
  'render_image_from_bytes|Arc::unwrap_or_clone|Image::from_bytes' \
  apps/desktop/src

Repository: oxidezap/whatsapp-rust

Length of output: 6356


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,260p' apps/desktop/src/components/message_bubble.rs
printf '\n---\n'
sed -n '260,460p' apps/desktop/src/components/message_bubble.rs
printf '\n---\n'
sed -n '460,700p' apps/desktop/src/components/message_bubble.rs
printf '\n---\n'
sed -n '1488,1535p' apps/desktop/src/app/mod.rs

Repository: oxidezap/whatsapp-rust

Length of output: 28992


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C3 --type=rust 'struct MediaContent|enum MediaType|struct ChatMessage|media:|data: Arc<Vec<u8>>|sticker_images|get_sticker_image|Image::from_bytes|unwrap_or_clone' apps/desktop/src

Repository: oxidezap/whatsapp-rust

Length of output: 15312


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,180p' apps/desktop/src/components/message_list.rs

Repository: oxidezap/whatsapp-rust

Length of output: 4813


Cache the decoded Arc<Image> instead of rebuilding it in render_image_from_bytes. This path still falls back to a full byte clone and re-decodes the image on each render for image messages and sticker previews. Reuse a cached Arc<Image> here, like the sticker path already does, so the bubble can render the decoded image directly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/components/message_bubble.rs` around lines 390 - 400, The
render_image_from_bytes function currently clones image bytes and decodes the
image on every render. Update this path to obtain and reuse a cached Arc<Image>,
following the existing sticker-preview caching approach, and render the cached
decoded image directly while preserving the current format and sizing behavior.

Comment on lines +16 to +23
pub fn render_message_list(
cache: MessageListCache,
scroll_handle: &VirtualListScrollHandle,
entity: Entity<WhatsAppApp>,
_playing_message_id: Option<String>,
is_group: bool,
layout: ResponsiveLayout,
) -> impl IntoElement {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Drop the unused _playing_message_id parameter.

Look, if we're not using it, we shouldn't be shipping it — clean APIs matter. The parameter is dead on arrival since the closure re-reads app.playing_message_id() fresh every render (as the comment explains). Keeping it just makes callers pass a value nobody looks at.

♻️ Proposed cleanup
 pub fn render_message_list(
     cache: MessageListCache,
     scroll_handle: &VirtualListScrollHandle,
     entity: Entity<WhatsAppApp>,
-    _playing_message_id: Option<String>,
     is_group: bool,
     layout: ResponsiveLayout,
 ) -> impl IntoElement {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn render_message_list(
cache: MessageListCache,
scroll_handle: &VirtualListScrollHandle,
entity: Entity<WhatsAppApp>,
_playing_message_id: Option<String>,
is_group: bool,
layout: ResponsiveLayout,
) -> impl IntoElement {
pub fn render_message_list(
cache: MessageListCache,
scroll_handle: &VirtualListScrollHandle,
entity: Entity<WhatsAppApp>,
is_group: bool,
layout: ResponsiveLayout,
) -> impl IntoElement {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/components/message_list.rs` around lines 16 - 23, Remove the
unused _playing_message_id parameter from render_message_list and update every
call site to stop passing it, while preserving the closure’s existing use of
app.playing_message_id() for each render.

Comment on lines +27 to +28
/// NAL length size in AVCC format (typically 4 bytes for WhatsApp videos)
const NAL_LENGTH_SIZE: usize = 4;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'NAL_LENGTH_SIZE|avcc_to_annexb|length_size_minus_one' \
  apps/desktop/src/video/streaming.rs apps/desktop/Cargo.toml

Repository: oxidezap/whatsapp-rust

Length of output: 482


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- streaming.rs excerpt around AVCC handling ---'
sed -n '240,330p' apps/desktop/src/video/streaming.rs

echo
echo '--- avcc_to_annexb implementation ---'
sed -n '560,650p' apps/desktop/src/video/streaming.rs

echo
echo '--- Cargo dependency hints ---'
sed -n '1,220p' apps/desktop/Cargo.toml | rg -n 'mp4|avc|h264|video'

echo
echo '--- search for length size / avcC usage across repo ---'
rg -n 'lengthSizeMinusOne|length_size_minus_one|avcC|AVCC|NAL_LENGTH_SIZE|avcc_to_annexb' apps/desktop/src apps/desktop/Cargo.toml

Repository: oxidezap/whatsapp-rust

Length of output: 6467


🌐 Web query:

mp4 crate avcC lengthSizeMinusOne sample conversion Rust

💡 Result:

In the context of the ISO Base Media File Format (MP4), the avcC (AVCDecoderConfigurationRecord) box contains an AVCLevelIndication field followed by reserved bits and the lengthSizeMinusOne field [1][2][3]. The lengthSizeMinusOne field is a 2-bit value that specifies the number of bytes used to indicate the length of each NAL unit in the sample data [1][4][3]. To convert lengthSizeMinusOne to the actual length size (the number of bytes for the length prefix), add 1 to the value of the 2-bit field [1][5][3]. Common values are: - 0: 1-byte length prefix - 1: 2-byte length prefix - 3: 4-byte length prefix (most common) [2][3] In Rust, this conversion is typically performed using bitwise operations on the byte containing the field [1][4][5]. Assuming the byte is extracted from the avcC record: let length_size = ((byte_containing_field & 0x03) + 1) as usize; [1][3] When processing MP4 video samples (found in mdat boxes), you must use this length_size to correctly parse the length-prefixed NAL units that follow [1][2]. Libraries such as rust_h264 provide utilities to handle this, such as parse_avcc_config to extract the configuration and parse_avcc to parse the samples using the determined length_size [2][6][7]. The SPS and PPS parameter sets are stored out-of-band in the avcC box and must be parsed and fed to the decoder before any sample NALs are processed [1][2][7].

Citations:


Read the NAL length width from avcC, don’t hardcode 4 bytes. AVCC allows 1-, 2-, or 4-byte NAL prefixes, so this will misparse valid MP4 samples. Pass the track’s lengthSizeMinusOne into avcc_to_annexb and reject malformed records.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/video/streaming.rs` around lines 27 - 28, Update
avcc_to_annexb to accept the track’s avcC lengthSizeMinusOne value and derive
the NAL prefix width as 1, 2, or 4 bytes instead of using NAL_LENGTH_SIZE. Read
and pass this value from the video track metadata, validate malformed avcC
records and NAL lengths, and reject unsupported or inconsistent widths before
parsing samples.

Comment on lines +129 to +131
// Get video dimensions
let width = video_track.width() as u32;
let height = video_track.height() as u32;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound video dimensions before allocating the RGBA buffer.

Downloaded media controls width and height, but Line 248 allocates their unchecked product. Reject zero or excessive pixel counts and use checked multiplication to prevent overflow or process-killing allocations.

Proposed guard
+        const MAX_VIDEO_PIXELS: usize = 7680 * 4320;
         let width = video_track.width() as u32;
         let height = video_track.height() as u32;
+
+        let pixel_count = (width as usize)
+            .checked_mul(height as usize)
+            .filter(|&pixels| pixels > 0 && pixels <= MAX_VIDEO_PIXELS)
+            .ok_or_else(|| anyhow!("Unsupported video dimensions: {width}x{height}"))?;
...
-        let rgba_byte_len = (width as usize) * (height as usize) * 4;
+        let rgba_byte_len = pixel_count
+            .checked_mul(4)
+            .context("RGBA buffer size overflow")?;

Also applies to: 217-235, 248-249

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/video/streaming.rs` around lines 129 - 131, Validate the
dimensions obtained in the video streaming path before the RGBA buffer
allocation: reject zero dimensions and pixel counts above the supported maximum,
then use checked multiplication when computing the buffer size. Update the
allocation logic around the video dimension handling and the later RGBA buffer
creation so invalid or overflowing sizes return an appropriate error instead of
allocating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants